lib/checkout: Fix regression in subpath for regular files

This is what caused the merge of
https://github.com/projectatomic/rpm-ostree/pull/652
to blow up, since https://github.com/ostreedev/ostree/pull/848
landed right before we tried to merge it.

When I was writing that PR I remember having an uncertain feeling
since we were doing a `mkdirat` above, but at the time I thought
we'd have test suite coverage...turns out we didn't.

For backwards compatibility, we need to continue to do a `mkdirat` here of the
parent. However...I can't think of a reason anyone would *want* that behavior.
Hence, let's add a special trick - if the destination name is `.`, we skip
`mkdirat()`. That way rpm-ostree for example can open a dfd for `/etc` and avoid
the `mkdir`.

Fold the subpath tests into `test-basic.sh` since it's not worth a separate
file. Add a test case for checking out a file.

Closes: #854
Approved by: jlebon

diff --git a/Makefile-tests.am b/Makefile-tests.am
index 335ba9b7bdec9ccff93a4008e5d2a7e3b9e2cbed..89675288677999a1f65002ae60f76714f2f67f6b 100644 (file)
--- a/Makefile-tests.am
+++ b/Makefile-tests.am
@@ -96,7 +96,6 @@ _installed_or_uninstalled_test_scripts = \
        tests/test-admin-pull-deploy-split.sh \
        tests/test-admin-locking.sh \
        tests/test-admin-deploy-clean.sh \
-       tests/test-repo-checkout-subpath.sh     \
        tests/test-reset-nonlinear.sh \
        tests/test-oldstyle-partial.sh \
        tests/test-delta.sh \

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index f5ef9517b17ffb023ccfe2bd76f742be61df8c4e..236f514b1f097c8c58d26790ea26fa4a102c25a0 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -812,11 +812,31 @@ checkout_tree_at (OstreeRepo                        *self,
 
   /* Special case handling for subpath of a non-directory */
   if (g_file_info_get_file_type (source_info) != G_FILE_TYPE_DIRECTORY)
-    return checkout_one_file_at (self, options, &state,
-                                 ostree_repo_file_get_checksum (source),
-                                 destination_parent_fd,
-                                 g_file_info_get_name (source_info),
-                                 cancellable, error);
+    {
+      /* For backwards compat reasons, we do a mkdir() here. However, as a
+       * special case to allow callers to directly check out files without an
+       * intermediate root directory, we will skip mkdirat() if
+       * `destination_name` == `.`, since obviously the current directory
+       * exists.
+       */
+      int destination_dfd = destination_parent_fd;
+      glnx_fd_close int destination_dfd_owned = -1;
+      if (strcmp (destination_name, ".") != 0)
+        {
+          if (mkdirat (destination_parent_fd, destination_name, 0700) < 0
+              && errno != EEXIST)
+            return glnx_throw_errno_prefix (error, "mkdirat");
+          if (!glnx_opendirat (destination_parent_fd, destination_name, TRUE,
+                               &destination_dfd_owned, error))
+            return FALSE;
+          destination_dfd = destination_dfd_owned;
+        }
+      return checkout_one_file_at (self, options, &state,
+                                   ostree_repo_file_get_checksum (source),
+                                   destination_dfd,
+                                   g_file_info_get_name (source_info),
+                                   cancellable, error);
+    }
 
   /* Cache any directory metadata we read during this operation;
    * see commit b7afe91e21143d7abb0adde440683a52712aa246

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index 6ddf7b2e544f8f0fecb2f3d394e4157b7cd9f83f..b209b8390dea1f3576b265c1b81c3fddd0ed960a 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -341,6 +341,18 @@ cd ${test_tmpdir}
 $OSTREE checkout --subpath /yet/another test2 checkout-test2-subpath
 cd checkout-test2-subpath
 assert_file_has_content tree/green "leaf"
+cd ${test_tmpdir}
+rm checkout-test2-subpath -rf
+$OSTREE ls -R test2
+# Test checking out a file
+$OSTREE checkout --subpath /baz/saucer test2 checkout-test2-subpath
+assert_file_has_content checkout-test2-subpath/saucer alien
+# Test checking out a file without making a subdir
+mkdir t
+cd t
+$OSTREE checkout --subpath /baz/saucer test2 .
+assert_file_has_content saucer alien
+rm t -rf
 echo "ok checkout subpath"
 
 cd ${test_tmpdir}

diff --git a/tests/test-repo-checkout-subpath.sh b/tests/test-repo-checkout-subpath.sh
deleted file mode 100755 (executable)
index 2da5adc..0000000
--- a/tests/test-repo-checkout-subpath.sh
+++ /dev/null
@@ -1,35 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2011,2013 Colin Walters <walters@verbum.org>
-# Copyright (C) 2014 Red Hat, Inc.
-#
-# This library is free software; you can redistribute it and/or
-# modify it under the terms of the GNU Lesser General Public
-# License as published by the Free Software Foundation; either
-# version 2 of the License, or (at your option) any later version.
-#
-# This library is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public
-# License along with this library; if not, write to the
-# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
-# Boston, MA 02111-1307, USA.
-
-set -euo pipefail
-
-. $(dirname $0)/libtest.sh
-
-setup_test_repository "bare"
-
-echo '1..1'
-
-repopath=${test_tmpdir}/ostree-srv/gnomerepo
-
-${CMD_PREFIX} ostree --repo=repo checkout -U --subpath=/ test2 checkedout
-
-${CMD_PREFIX} ostree --repo=repo checkout -U --subpath=/firstfile test2 checkedout2
-
-echo "ok"